Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: _MACOSX directory in ootr archive can cause issues loading tracks #2300

Open
wants to merge 3 commits into
base: Dev
Choose a base branch
from

Conversation

EricBartusch
Copy link

In the Darunia's Joy discord, there was a user who was getting an error reading the .meta file for one of their songs.

I eventually figured out that it was because the ootrs archive also contained the author's _MACOSX dir and a ._<filename>.meta that was being read instead of the .meta file at the root.

I think an easy way to fix this for future users, besides having authors making sure their ootrs archives only have files, is to add a check in Music.py so only root files are processed:

@fenhl fenhl added Type: Bug Something isn't working Status: Needs Review Someone should be looking at it Status: Needs Testing Probably should be tested Component: Cosmetics Affects the patching of cosmetics labels Sep 9, 2024
@TreZc0
Copy link
Member

TreZc0 commented Sep 9, 2024

While we should add a failsafe on our end, does it not make a lot more sense to add shadow files from mac (or windows), like both of these, to the .gitignore of DJ so they are not committed in the first place?

@EricBartusch
Copy link
Author

EricBartusch commented Sep 9, 2024

Unfortunately, the file exists within the .ootrs archive which is just a renamed zip, so a .gitignore won't catch it.

Maybe a workflow could be added to unzip the archive and check for that on PR?
edit: DaruniasJoy/OoT-Custom-Sequences#460

@DaruniasJoy
Copy link

ahh... that is a reoccuring thing though rarely. did merge that pr on our side.

Damn Apple Users and bringing us issues. NotLikeThis

@fenhl fenhl removed the Status: Needs Review Someone should be looking at it label Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Cosmetics Affects the patching of cosmetics Status: Needs Testing Probably should be tested Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants